Skip to content

Add dict method for converting report data to dictionary#2188

Merged
jeromedockes merged 6 commits into
skrub-data:mainfrom
m4nn2609-dot:patch-1
Jul 2, 2026
Merged

Add dict method for converting report data to dictionary#2188
jeromedockes merged 6 commits into
skrub-data:mainfrom
m4nn2609-dot:patch-1

Conversation

@m4nn2609-dot

Copy link
Copy Markdown
Contributor

closes #2147
Add a dict() method to TableReport that returns the report data as a
Python dictionary, equivalent to json.loads(report.json()).

This allows users to access the report contents programmatically without
having to manually parse the JSON string.

Added a dict method to convert report data to a Python dictionary format.
@rcap107

rcap107 commented Jun 24, 2026

Copy link
Copy Markdown
Member

Hi @m4nn2609-dot, thanks for this PR. I saw you asked to work on this issue, but did not wait for a maintainer before opening a PR. For this time we'll let it slide, but if it happens again in the future we may close the PR without reviewing it.

There are various reasons why we'd rather assign issues explicitly: there may already be an open PR dealing with the issue, the issue may not be detailed enough to work on, or we may have decided to not work on the issue after all. If you start working on the issue without asking, there is the risk that the PR gets closed anyway and you waste your time.

It was not the case for this particular PR, but please keep it in mind for future contributions.

@m4nn2609-dot

Copy link
Copy Markdown
Contributor Author

Thanks for the heads up! I'll make sure to wait for explicit assignment before opening a PR next time. Appreciate the patience .Sorry for the unprofessional behavior from my side.

@rcap107

rcap107 commented Jun 24, 2026

Copy link
Copy Markdown
Member

Thanks for the heads up! I'll make sure to wait for explicit assignment before opening a PR next time. Appreciate the patience .Sorry for the unprofessional behavior from my side.

No worries! This kind of things happens all the time, but very often who opens the PR doesn't take the time to actually respond to our comments.

Regarding your PR, it's failing because it needs an entry in the changelog, the code was not formatted and it's missing a test for the new line.

For the test you can test that the output of the function is the same as the expected output (which basically means taking the code that generates the json in the first place and use that to prepare the expected)

You can find how to address the other two problems in the contributing guide

@m4nn2609-dot

m4nn2609-dot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

please review my pr
@rcap107

@rcap107 rcap107 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks a lot @m4nn2609-dot

@jeromedockes jeromedockes merged commit 8913e53 into skrub-data:main Jul 2, 2026
29 checks passed
@m4nn2609-dot m4nn2609-dot deleted the patch-1 branch July 3, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add a TableReport.dict() to get the content of json()

3 participants